Ticketing support #279
Ticketing support #279
Conversation
| List tickets | ||
|
|
||
| Options: | ||
| --status=open or closed Display only opened or closed tickets, otherwise display both |
There was a problem hiding this comment.
I think there's different docopt syntax that can be used here to better indicate the possible options. Have you tried this?
--status=open|closed
|
Looks like you also need to add some unit tests for the new manager. I'd also encourage you to try some integration and functional tests for the CLI if you've got the time. Otherwise, I'm really liking what I see here and most of my notes are more stylistic than anything else. |
|
Hey Nathan, thanks for your comments, I will try to work on them today or tomorrow. |
|
Sorry it took a while, this should be fine now. I need to figure out how to take care of those pep8 issues on my local machine. |
| if ticket['assignedUser']: | ||
| t.add_row([ | ||
| ticket['id'], | ||
| ticket['assignedUser']['firstName'] + |
There was a problem hiding this comment.
String concatenation isn't typically preferred in Python for the sake of readability. Usually you would do something like this in Python:
"%s %s" % (ticket['assignedUser']['firstName'],
ticket['assignedUser']['lastName'])
| 'assignedUserId': currentUser['id'], | ||
| 'title': title, | ||
| } | ||
| # if (hardware is None): |
There was a problem hiding this comment.
Could this commented out code be cleaned up? :)
|
Thanks for all your comments, you can tell im new to this. Will get those fixed soon. |
|
I "believe" I captured all your comments. wissam |
|
|
||
| Required: | ||
| --title=TITLE The title of the ticket | ||
| --subject=xxx The id of the subject to use for the ticket, |
There was a problem hiding this comment.
Usually, tags are all capitalized and describe what's required. For this case I think ID fits.
usage: sl ticket create --title=TITLE --subject=ID [options]
and
--subject=ID The id of the subject to use for the ticket,
|
@anasouma this is looking really great. After addressing my last round of comments I think it'll be good to pull. |
|
|
||
| :param string title: title for the new ticket | ||
| :param string body: body for the new ticket | ||
| :param string hardware: id of the hardware to be assigned to the ticket |
There was a problem hiding this comment.
this should match the parameter list. It's subject, and I'm guessing this could apply to CCI or hardware? I'm unsure.
|
Thanks for your comments, I made all the changes you requested. |
|
LGTM, +1; one more person (@beittenc or @jasonjohnson)? will need to +1 this before merging according to the process we have. |
|
+1, looks good to me as well. Thanks for all the work on this module. |
No description provided.